Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TECH] Configuration des workspaces pnpm #10838

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft

[TECH] Configuration des workspaces pnpm #10838

wants to merge 7 commits into from
+29,307 −268,305

Conversation

bpetetot
Copy link
Contributor

@bpetetot bpetetot commented Dec 17, 2024 •
edited
Loading

🎄 Problème

🎁 Proposition

Travail basé sur la PR #7155

🗒️ TODO

  • Mettre à jour:
    • INSTALLATION.md
    • README.md de chaque app Pix
    • procfile de scalingo (postdeploy)
    • configure.sh
    • les fichier Dockerfile, docker-compose dans le dossier docker
    • scripts utilisent maintenant pnpm au lieu de npm.
    • Mettre à jour les configurations .vscode
    • A voir si c'est possible de remplacer npm-run-all
  • Appliquer la modification de ce commit pour faire fonctionner la CI.
  • Reprendre le script de migration de la PR [TECH] Utiliser pnpm #7155, le rendre cross-plateforme (utile pour la migration d'autre repo ou la mise à jour de cette branche)

🧦 Documentation

  • Le commit ajoutant la propriété packageManager dans le package.json n'a pas été repris car il ne semble pas nécessaire d'après la documentation (bcf82f5)
  • Le commit concernant Bookshelf n'a pas été repris car la librarie a été supprimée de l'API (fca31e7)
  • Le buildpack nodejs de Scalingo supporte désormais pnpm par défaut. Donc il n'y plus besoin d'avoir un fork du buildpack comme dans la PR originale (https://github.com/Scalingo/nodejs-buildpack/blob/master/CHANGELOG.md#v245---2024-04-29)
  • Le commit modifiant la CI sera fait à la fin car on veut d'abord que ça fonctionne en local (ab06ad1)
  • Le commit corrigeant le build en supprimant polyfillUUID n'a pour le moment pas été repris et sera fait plus tard (voir cette PR [TECH] Suppression de polyfills Intl (PIX-13595) #9820 pour exemple)
  • Le script migrate-to-pnpm a été lançé mais non commité.
    • Sur Mac, nous avons dû modifier le script migrate-to-pnpm.sh pour faire marcher les commandes sed en ajoutant une chaîne vide ('') après le -i.

Important

Il est préférable de partir d'une "clean install" (sans node modules), sinon il faut au préalable les supprimer (dans tous les projets).

🚨 Il est nécessaire de supprimer le répertoire temporaire $TMPDIR/embroider

🧰 Questions / Améliorations

  • Dans le package.json racine:
    • la commande "build" ne fait le build que des fronts et pas audit logger. Pourquoi ?
    • Pourquoi les commandes lint et test utilisent des regex ?
  • Raccourcir les commandes avec pnpm run (run non obligatoire)

🎅 Pour tester

Fonctionnement des commandes racines:

  • pnpm install
  • pnpm lint
  • pnpm test
  • pnpm dev

Commandes admin

  • pnpm dev

Commandes api

  • pnpm dev
  • pnpm lint
  • pnpm test
  • pnpm db:reset

Commandes audit-logger

  • pnpm dev

Commandes certif

  • pnpm dev

Commandes junior

  • pnpm dev

Commandes mon-pix

  • pnpm dev

Commandes orga

  • pnpm dev

@er-lim er-lim marked this pull request as draft December 17, 2024 13:43
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@er-lim er-lim added Development in progress cross-team Toutes les équipes de dev labels Dec 17, 2024
@bpetetot bpetetot force-pushed the tech-return-of-pnpm-workspaces branch 2 times, most recently from 0d3d8a5 to 8fa0f00 Compare December 17, 2024 15:27
bpetetot
bpetetot commented Dec 18, 2024
View reviewed changes
package.json
Comment on lines +43 to +46
"test": "pnpm /^test:.*/",
"test:packages": "pnpm --filter test --workspaces --if-present",
"test:api": "pnpm --filter api lint && pnpm --filter api test",
"test:api:debug": "pnpm --filter api test:api:debug",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ne fonctionne pas

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

il faut utiliser le nom du package et pas le nom du répertoire, donc pix-api dans ce cas

@bpetetot bpetetot force-pushed the tech-return-of-pnpm-workspaces branch from 735c1b2 to e007a87 Compare December 18, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HEYGUL HEYGUL HEYGUL left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@bpetetot bpetetot

@er-lim er-lim

Labels
cross-team Toutes les équipes de dev Development in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bpetetot @pix-bot-github @HEYGUL @er-lim @francois2metz